REPORT-912: Update to OpenMRS platform 2.7.3 and Support Java 21 #261

wikumChamith · 2025-05-30T06:02:30Z

Ticket: https://openmrs.atlassian.net/browse/REPORT-912

legacy-branch: https://github.com/openmrs/openmrs-module-reporting/tree/1.x

dkayiwa · 2025-05-30T09:21:49Z

Can we add include the other Java versions in the build?

dkayiwa · 2025-05-30T09:31:54Z

...va/org/openmrs/module/reporting/dataset/definition/evaluator/DataExportDataSetEvaluator.java

+//import org.openmrs.util.OpenmrsUtil;
+//
+///**
+// * The logic that evaluates a {@link DataExportDataSetDefinition} and produces a {@link DataSet}


What is this?

This is a deprecated class. So I am deleting this.

Did you see the above comment?

Yeah. Sorry, my mistake. I am working on this.

dkayiwa · 2025-05-30T09:32:19Z

api-1.9/src/main/java/org/openmrs/module/reporting/report/service/db/PropertiesType.java

-		return x != null && x.equals(y);
-	}
-
-	/** 


Are these formatting changes?

I replaced this with PropertiesType in api-2.4. We have to remove this when removing the sub-module.

https://github.com/openmrs/openmrs-module-reporting/blob/master/api-2.4/src/main/java/org/openmrs/module/reporting/report/service/db/PropertiesType.java

Before removing the submodule, can we avoid touching this?

@dkayiwa We need to update the nullSafeGet and nullSafeSet methods; without doing so, the code won't compile. I reverted the file temporarily, and as you can see from the build, it's failing with compile errors:
https://github.com/openmrs/openmrs-module-reporting/actions/runs/15436424952/job/43443786399

dkayiwa · 2025-05-30T09:32:49Z

...sts/src/test/java/org/openmrs/module/reporting/definition/service/DefinitionServiceTest.java

-		assertThat(persistedDefinition, notNullValue());
-		assertThat(persistedDefinition.getName(), is("Patients in 2006 by indicators"));
-		assertThat(persistedDefinition.getSpecifications(), not(empty()));
+//		executeDataSet("org/openmrs/module/reporting/include/DefinitionServiceTest.xml");


What is this?

This is a deprecated class. So I am deleting this.

dkayiwa · 2025-05-30T09:33:14Z

api-1.9/src/test/java/org/openmrs/module/reporting/dataset/DataExportDataSetPersisterTest.java

+//import org.openmrs.reporting.export.DataExportReportObject;
+//import org.openmrs.test.BaseModuleContextSensitiveTest;
+//import org.openmrs.test.StartModule;
+//


This is a deprecated class. So I am deleting this.

dkayiwa · 2025-05-30T09:33:26Z

api-1.9/src/main/java/org/openmrs/module/reporting/report/service/db/ReportDefinitionType.java

-	/** 
-	 * @see UserType#equals(Object, Object)
-	 */
-	public boolean equals(Object x, Object y) throws HibernateException {


Formatting changes again?

I just replaced this with ReportDefinitionType in api-2.4. We have to remove this when removing the sub-module.

https://github.com/openmrs/openmrs-module-reporting/blob/master/api-2.4/src/main/java/org/openmrs/module/reporting/report/service/db/ReportDefinitionType.java

I just do not seem to understand what is going on here. I see it deleted and added back in the same file.

dkayiwa · 2025-05-30T09:33:37Z

api-1.9/src/main/java/org/openmrs/module/reporting/report/service/db/RenderingModeType.java

-import org.hibernate.usertype.CompositeUserType;
-import org.hibernate.usertype.UserType;
-import org.openmrs.module.reporting.common.HibernateUtil;
-import org.openmrs.module.reporting.report.renderer.RenderingMode;


Formatting changes?

I just replaced this with RenderingModeType in api-2.4. We have to remove this when removing the sub module.

openmrs-module-reporting/api-2.4/src/main/java/org/openmrs/module/reporting/report/service/db/RenderingModeType.java

Line 38 in 5419198

public class RenderingModeType implements CompositeUserType {

dkayiwa · 2025-05-30T12:18:11Z

api-1.9/pom.xml

+        <dependency>
+            <groupId>org.openmrs.module</groupId>
+            <artifactId>reportingcompatibility-api</artifactId>
+            <version>3.0.0-SNAPSHOT</version>


Did you forget to add scope here?

dkayiwa · 2025-05-30T12:27:08Z

api-1.9/src/main/java/org/openmrs/module/reporting/report/service/db/PropertiesType.java

+	/** 
+	 * @see UserType#assemble(Serializable, Object)
+	 */
+	public Object assemble(Serializable cached, Object owner) throws HibernateException {


Is there where you added it after deleting it from the above?

dkayiwa · 2025-05-30T12:27:48Z

api-1.9/src/main/java/org/openmrs/module/reporting/report/service/db/RenderingModeType.java

+public class RenderingModeType implements CompositeUserType {
+
+	/**
+	 * @see CompositeUserType#returnedClass()


Is this deleting and then adding it back as new?

@dkayiwa, what's happening here is that method signatures like nullSafeGet have changed in newer platform versions. Specifically:

CompositeUserType#nullSafeGet(ResultSet, String[], SharedSessionContractImplementor, Object)

has been updated to:

CompositeUserType#nullSafeGet(ResultSet, String[], SharedSessionContractImplementor, Object)

Previously, I took a shortcut by copying these classes from the 2.4 branch, since we're planning to remove these classes when we eventually merge the legacy submodules. I've now updated the PR to avoid confusion.

wikumChamith · 2025-05-30T13:03:14Z

api-1.9/src/main/java/org/openmrs/module/reporting/report/service/db/MappedDefinitionType.java

+	public Object nullSafeGet(ResultSet rs, String[] names, SharedSessionContractImplementor session, Object owner) throws HibernateException, SQLException {
+		String parameterizableUuid = (String) HibernateUtil.standardType("STRING").nullSafeGet(rs, names[0], session, owner);
+		if (StringUtils.isEmpty(parameterizableUuid)) { return null; }
+		String serializedMappings = (String) HibernateUtil.standardType("STRING").nullSafeGet(rs, names[1], session, owner);
+		Definition d = DefinitionContext.getDefinitionByUuid(mappedType, parameterizableUuid);
+		Map<String, Object> mappings = new HashMap<String, Object>();
+		if (StringUtils.isNotBlank(serializedMappings)) {
+			try {
+				mappings = Context.getSerializationService().deserialize(serializedMappings, Map.class, ReportingSerializer.class);
+			}
+			catch (Exception e) {
+				throw new HibernateException("Unable to deserialize parameter mappings for definition", e);
+			}
+		}
+		return new Mapped(d, mappings);
+	}
+
+	/**
+	 * @see CompositeUserType#nullSafeSet(PreparedStatement, Object, int, SharedSessionContractImplementor)
+	 */
+	public void nullSafeSet(PreparedStatement st, Object value, int index, SharedSessionContractImplementor session) throws HibernateException, SQLException {
+		String definitionUuid = null;


I don't know why GitHub is showing this as a completely new file. The only thing I did was change these methods :).

When comparing these changes locally, I am not seeing this issue.

dkayiwa · 2025-06-02T12:09:15Z

api/pom.xml

 			<groupId>org.openmrs.module</groupId>
 			<artifactId>reportingcompatibility-api</artifactId>
 			<version>${reportingCompatibilityVersion}</version>
-			<scope>provided</scope>


Why did you change this scope?

I am getting an error with the 'provided' scope.

java.lang.IllegalStateException: Failed to load ApplicationContext at org.springframework.test.context.cache.DefaultCacheAwareContextLoaderDelegate.loadContext(DefaultCacheAwareContextLoaderDelegate.java:98) at org.springframework.test.context.support.DefaultTestContext.getApplicationContext(DefaultTestContext.java:124) at org.springframework.test.context.support.DependencyInjectionTestExecutionListener.injectDependencies(DependencyInjectionTestExecutionListener.java:118) at org.springframework.test.context.support.DependencyInjectionTestExecutionListener.prepareTestInstance(DependencyInjectionTestExecutionListener.java:83) at org.springframework.test.context.TestContextManager.prepareTestInstance(TestContextManager.java:248) at org.springframework.test.context.junit4.SpringJUnit4ClassRunner.createTest(SpringJUnit4ClassRunner.java:227) at org.springframework.test.context.junit4.SpringJUnit4ClassRunner$1.runReflectiveCall(SpringJUnit4ClassRunner.java:289) at org.springframework.test.context.junit4.SpringJUnit4ClassRunner.methodBlock(SpringJUnit4ClassRunner.java:291) at org.springframework.test.context.junit4.SpringJUnit4ClassRunner.runChild(SpringJUnit4ClassRunner.java:246) at org.springframework.test.context.junit4.SpringJUnit4ClassRunner.runChild(SpringJUnit4ClassRunner.java:97) at org.springframework.test.context.junit4.statements.RunBeforeTestClassCallbacks.evaluate(RunBeforeTestClassCallbacks.java:61) at org.springframework.test.context.junit4.statements.RunAfterTestClassCallbacks.evaluate(RunAfterTestClassCallbacks.java:70) at org.springframework.test.context.junit4.SpringJUnit4ClassRunner.run(SpringJUnit4ClassRunner.java:190) Caused by: org.springframework.beans.factory.BeanCreationException: Error creating bean with name 'dataExportDataSetDefinitionPersister': Lookup method resolution failed; nested exception is java.lang.IllegalStateException: Failed to introspect Class [org.openmrs.module.reporting.dataset.definition.persister.DataExportDataSetDefinitionPersister] from ClassLoader [sun.misc.Launcher$AppClassLoader@75b84c92] at org.springframework.beans.factory.annotation.AutowiredAnnotationBeanPostProcessor.determineCandidateConstructors(AutowiredAnnotationBeanPostProcessor.java:298) at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.determineConstructorsFromBeanPostProcessors(AbstractAutowireCapableBeanFactory.java:1302) at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.createBeanInstance(AbstractAutowireCapableBeanFactory.java:1219) at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.doCreateBean(AbstractAutowireCapableBeanFactory.java:582) at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.createBean(AbstractAutowireCapableBeanFactory.java:542) at org.springframework.beans.factory.support.AbstractBeanFactory.lambda$doGetBean$0(AbstractBeanFactory.java:335) at org.springframework.beans.factory.support.DefaultSingletonBeanRegistry.getSingleton(DefaultSingletonBeanRegistry.java:234) at org.springframework.beans.factory.support.AbstractBeanFactory.doGetBean(AbstractBeanFactory.java:333) at org.springframework.beans.factory.support.AbstractBeanFactory.getBean(AbstractBeanFactory.java:208) at org.springframework.beans.factory.support.DefaultListableBeanFactory.preInstantiateSingletons(DefaultListableBeanFactory.java:955) at org.springframework.context.support.AbstractApplicationContext.finishBeanFactoryInitialization(AbstractApplicationContext.java:921) at org.springframework.context.support.AbstractApplicationContext.refresh(AbstractApplicationContext.java:583) at org.springframework.test.context.support.AbstractGenericContextLoader.loadContext(AbstractGenericContextLoader.java:127) at org.springframework.test.context.support.AbstractGenericContextLoader.loadContext(AbstractGenericContextLoader.java:60) at org.springframework.test.context.support.AbstractDelegatingSmartContextLoader.delegateLoading(AbstractDelegatingSmartContextLoader.java:276) at org.springframework.test.context.support.AbstractDelegatingSmartContextLoader.loadContext(AbstractDelegatingSmartContextLoader.java:244) at org.springframework.test.context.cache.DefaultCacheAwareContextLoaderDelegate.loadContextInternal(DefaultCacheAwareContextLoaderDelegate.java:141) at org.springframework.test.context.cache.DefaultCacheAwareContextLoaderDelegate.loadContext(DefaultCacheAwareContextLoaderDelegate.java:90) ... 12 more Caused by: java.lang.IllegalStateException: Failed to introspect Class [org.openmrs.module.reporting.dataset.definition.persister.DataExportDataSetDefinitionPersister] from ClassLoader [sun.misc.Launcher$AppClassLoader@75b84c92] at org.springframework.util.ReflectionUtils.getDeclaredMethods(ReflectionUtils.java:485) at org.springframework.util.ReflectionUtils.doWithLocalMethods(ReflectionUtils.java:321) at org.springframework.beans.factory.annotation.AutowiredAnnotationBeanPostProcessor.determineCandidateConstructors(AutowiredAnnotationBeanPostProcessor.java:276) ... 29 more Caused by: java.lang.NoClassDefFoundError: org/openmrs/reporting/AbstractReportObject at java.lang.Class.getDeclaredMethods0(Native Method) at java.lang.Class.privateGetDeclaredMethods(Class.java:2729) at java.lang.Class.getDeclaredMethods(Class.java:2003) at org.springframework.util.ReflectionUtils.getDeclaredMethods(ReflectionUtils.java:467) ... 31 more Caused by: java.lang.ClassNotFoundException: org.openmrs.reporting.AbstractReportObject at java.net.URLClassLoader.findClass(URLClassLoader.java:387) at java.lang.ClassLoader.loadClass(ClassLoader.java:418) at sun.misc.Launcher$AppClassLoader.loadClass(Launcher.java:352) at java.lang.ClassLoader.loadClass(ClassLoader.java:351) ... 35 more

We can't have this scope changed from provided to compile.

We can't have this scope changed from provided to compile.

@mseaton, do you know why this error is occurring?

Yes @wikumChamith . Currently, this class is in the api-1.9 maven submodule, which builds against OpenMRS 1.9 and is only conditionally loaded by OpenMRS if the core version is 1.9-1.12:

<conditionalResource> <path>/lib/reporting-api-1.9.*</path> <openmrsVersion>1.9.9 - 1.12.*</openmrsVersion> </conditionalResource>

Now that you are trying to move this class into the main api maven submodule and have it build against OpenMRS 2.x, it is failing because the classes that it depends upon are no longer in core, but only found in the reportingcompatibility module.

Best thing to do is to try to get the DataExportDataSetEvaluator and DataExportDataSetDefinitionPersister to be conditionally loaded only if the reportingcompatibility module is running, and to make reportingcompatibility an aware-of dependency in config.xml, and a provided dependency in the pom, but I'm not sure that's possible to do with the Handler annotation.

We might also just remove support for these constructs from the reporting module, since I'm not confident that anyone actually ever used them, and it is even more likely that no one is currently using them.

@dkayiwa / @ibacher ?

I am in favour of removing them to make things simpler. If any one ever complains about them, we can then look into bringing them back.

Removing those two classes fixed the issue.

Great - can you remove the reportingcompatibility dependency entirely then?

Removed it.

dkayiwa · 2025-06-04T21:31:34Z

@wikumChamith would it require as many changes if we simply changed the platform version to 2.4.0?

You can clearly see that iam trying to do a step by step upgrade in order to limit the number of changes, especially for these complicated modules. 😊

wikumChamith · 2025-06-05T07:33:20Z

@wikumChamith would it require as many changes if we simply changed the platform version to 2.4.0?

You can clearly see that iam trying to do a step by step upgrade in order to limit the number of changes, especially for these complicated modules. 😊

@dkayiwa PR for 2.4 update: #262

wikumChamith · 2025-06-11T09:07:23Z

@dkayiwa This is now ready to review.

mseaton

See comments. We cannot change module dependencies from provided to compile scope.

Ruhanga

Overall, this is looking good — thanks @wikumChamith! I have a few suggestions that I believe would be a good fit to include in this PR.

...mrs/module/reporting/cohort/definition/evaluator/EncounterCohortDefinitionEvaluatorTest.java

...penmrs/module/reporting/query/encounter/evaluator/MappedParametersObsQueryEvaluatorTest.java

...t/java/org/openmrs/module/reporting/query/visit/evaluator/ActiveVisitQueryEvaluatorTest.java

...s/src/test/java/org/openmrs/module/reporting/data/converter/PrivilegedDataConverterTest.java

...rc/test/java/org/openmrs/module/reporting/data/visit/evaluator/VisitIdDataEvaluatorTest.java

wikumChamith · 2025-07-07T12:06:18Z

@Ruhanga, I’ve migrated the tests I touched in this PR to use Jupiter BaseModuleContextSensitiveTest. But if we’re planning to migrate all the tests in the module, I think it’s better to create a separate ticket.

Ruhanga · 2025-07-07T12:28:16Z

Correct and thanks @wikumChamith for the nice work.

Ruhanga

LGTM!

I guess we might need a release before this can get merged in. Any objections or recommendations @mseaton, @dkayiwa

mseaton · 2025-07-07T13:52:28Z

LGTM!

I guess we might need a release before this can get merged in. Any objections or recommendations @mseaton, @dkayiwa

Releasing existing version now.

Ruhanga

I'm happy with this. Any objections to having this merged in dear reviewers, @dkayiwa, @mseaton, @ibacher?

mseaton

Looks good to me, thanks!

dkayiwa · 2025-07-20T22:27:30Z

api-tests/src/test/java/org/openmrs/module/reporting/evaluation/EvaluationProfilerTest24On.java

- */
-@TestExecutionListeners(OpenmrsVersionTestListener.class)
-@RequiresVersion("2.4.* - 2.*")
-public class EvaluationProfilerTest24On extends BaseModuleContextSensitiveTest {


Why did we remove this class?

dkayiwa · 2025-07-20T22:32:42Z

...s/src/test/java/org/openmrs/module/reporting/data/converter/PrivilegedDataConverterTest.java

-        PowerMockito.when(Context.hasPrivilege(DOES_NOT_HAVE_PRIV)).thenReturn(false);
+        initializeInMemoryDatabase();
+        executeDataSet("org/openmrs/module/reporting/include/PrivilegeTest.xml");
+        Context.logout();


Do we really need to log out and authenticate?

wikumChamith · 2025-07-22T05:12:08Z

@dkayiwa I updated the PR.

wikumChamith marked this pull request as draft May 30, 2025 06:12

wikumChamith mentioned this pull request May 30, 2025

Update the module to Platform 2.7.0 #260

Closed

wikumChamith changed the title ~~Report-912: Update to OpenMRS platform 2.7.3 and Support Java 21~~ REPORT-912: Update to OpenMRS platform 2.7.3 and Support Java 21 May 30, 2025

dkayiwa reviewed May 30, 2025

View reviewed changes

wikumChamith requested a review from dkayiwa May 30, 2025 11:47

dkayiwa reviewed May 30, 2025

View reviewed changes

wikumChamith force-pushed the REPORT-912 branch from decf6c2 to 2ab70fc Compare May 30, 2025 12:55

wikumChamith commented May 30, 2025

View reviewed changes

wikumChamith requested a review from dkayiwa May 30, 2025 14:47

dkayiwa reviewed Jun 2, 2025

View reviewed changes

wikumChamith force-pushed the REPORT-912 branch from 318a7d2 to 69b590b Compare June 3, 2025 09:02

wikumChamith requested a review from dkayiwa June 3, 2025 10:31

wikumChamith force-pushed the REPORT-912 branch 5 times, most recently from 7d3a450 to 88d0b8b Compare June 4, 2025 07:36

wikumChamith force-pushed the REPORT-912 branch from 88d0b8b to b1f1c03 Compare June 11, 2025 07:45

wikumChamith marked this pull request as ready for review June 11, 2025 07:46

wikumChamith force-pushed the REPORT-912 branch from b1f1c03 to 11316ce Compare June 11, 2025 07:52

wikumChamith force-pushed the REPORT-912 branch 2 times, most recently from e68ddeb to 1179b0f Compare June 11, 2025 07:55

wikumChamith requested review from ibacher and mseaton June 11, 2025 09:07

mseaton requested changes Jun 11, 2025

View reviewed changes

wikumChamith force-pushed the REPORT-912 branch from a765130 to aa94c30 Compare June 11, 2025 14:49

wikumChamith requested a review from mseaton June 11, 2025 14:53

Ruhanga reviewed Jul 4, 2025

View reviewed changes

wikumChamith force-pushed the REPORT-912 branch from ccae528 to 27967a2 Compare July 4, 2025 12:54

Update to Platform 2.7.0 and Support Java 21

58cbd32

wikumChamith force-pushed the REPORT-912 branch from 28b860a to 58cbd32 Compare July 4, 2025 12:56

Merge branch 'master' into REPORT-912

a77e050

Ruhanga reviewed Jul 7, 2025

View reviewed changes

...s/src/test/java/org/openmrs/module/reporting/data/converter/PrivilegedDataConverterTest.java Outdated Show resolved Hide resolved

...rc/test/java/org/openmrs/module/reporting/data/visit/evaluator/VisitIdDataEvaluatorTest.java Outdated Show resolved Hide resolved

Migrate to org.openmrs.test.jupiter.BaseModuleContextSensitiveTest

0973e96

Ruhanga approved these changes Jul 7, 2025

View reviewed changes

Merge branch 'master' into REPORT-912

7a99af6

Ruhanga reviewed Jul 16, 2025

View reviewed changes

mseaton approved these changes Jul 18, 2025

View reviewed changes

dkayiwa reviewed Jul 20, 2025

View reviewed changes

Add the EvaluationProfilerTest24On

ca8eb8d

dkayiwa merged commit 2c8e57e into openmrs:master Jul 24, 2025
5 checks passed

REPORT-912: Update to OpenMRS platform 2.7.3 and Support Java 21 #261

REPORT-912: Update to OpenMRS platform 2.7.3 and Support Java 21 #261

Uh oh!

Conversation

wikumChamith commented May 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dkayiwa commented May 30, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wikumChamith May 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dkayiwa commented Jun 4, 2025

Uh oh!

wikumChamith commented Jun 5, 2025

wikumChamith commented May 30, 2025 •

edited

Loading

wikumChamith May 30, 2025 •

edited

Loading